Repository navigation
Revalidate the cache files on every request (0.3.2) - #24
Merged
Merged
Conversation
resetCache(), or any reset for an invalid cache, cancels the running download with the exception, and the download resets the cache when it ends on it. A download waiting for a response, or to retry, had no response for the exception to end, so it returned as if stopped and resumed the old partial cache. The download now ends on it either way, and a download cancelled for an invalid cache serves no more requests.
_cacheFileState checks the cache files against the running download: a partial cache file missing bytes the download saved (deleted underneath, by the device's cache management, say) is an invalid cache, and refreshCacheState resets it. request() refreshes the cache state every time, so a cache whose files were deleted or changed is downloaded again rather than failing the response, and the ad hoc deletion check in request() goes. While a download runs, the state takes its position (ahead of the file by the bytes it has yet to save) and source length, so a refresh no longer drops the position of a download of unknown length to zero, and the partial file is renamed only once the download stops writing to it. A reset of a download that stopped writing (completing, and refreshing on the way) is done at once rather than waiting for the download, which waits for the refresh.
A download resuming a partial cache file opens it to append when it first saves. Deleted since (appending creates it again), truncated or written to, the file took the download's bytes at the wrong offset, and the cache resumed from it later held them as the content: a complete cache file of the right size with the wrong bytes. The sink checks the file ends where it resumes it, and a file that does not is an invalid cache, which resets.
The reset came after the lock was released, so a refresh waiting for the lock could judge the same download again: two requests revalidating a deleted partial file reset the cache twice, and the late reset could delete the restarted download's file. The refresh that finds the cache invalid now cancels the download under the lock, so later refreshes see none to judge, and no request is served from it.
A resumed download whose partial file was deleted, receiving the rest at once, finished its response before its first save found the file gone: the failed save was reported as a plain error, the download waited 5 s to retry, and a file written to since would have been resumed from. The download now ends on the invalid cache found by a save after its response, or by a cancel while it saves its last bytes, as it does during a response, and resets at once. A download counts as writing until its sink is closed and the file released, not from when the sink starts closing: a refresh in between neither renames a file still open (which fails on Windows) nor resets the cache under the download, which then completed with no headers. While writing, it ends on the reset's exception instead. Once it stopped writing, the state is read from the files.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
HttpCacheStream.request()now revalidates the cache files against the running download on every call. A cache whose files were deleted or changed underneath it (by Android's cache management, say) is reset, and the request waits for its range to download again instead of its response failing. The deletion check #23 added torequest()moves into_cacheFileState(), which now runs it for every refresh. This is release 0.3.2: the pubspec is bumped and the CHANGELOG's Unreleased section becomes 0.3.2. Nothing is published.How it works
_cacheFileState()checks the partial file against the running download: the file must hold at least the bytes the download has saved (filePosition, read before the stat). A deleted or truncated file fails that check. So do the existing ones: a complete file of the wrong size, or a partial one longer than the source. Each throwsInvalidCacheSizeException, andrefreshCacheState()cancels the download it judged while still holding the file lock, then resets the cache.CacheState.filePosition. Once the download stops writing, the state is read from the files, and only then is the partial file renamed to complete.Bugs fixed along the way
BufferedIOSinknow checks, when it reopens a file to append, that the file ends where the sink resumes it. A file that doesn't is an invalid cache, which resets. The new test fails onmainwith a hash mismatch.resetCache()ignored between responses. A download waiting for the origin, or waiting to retry, had no open response for the reset's exception to end. It returned as if stopped and resumed the old partial cache.CacheDownloadernow ends on an invalid-cache cancel wherever it arrives (between responses, while saving its last bytes, or from a failed save) and serves no more requests after it. The new test fails onmain: the download resumes from the old partial.main's stream code.Behaviour changes
CacheStategainsfilePosition. It is optional onCacheState.incompleteand defaults toposition, and incomplete states compare by it too.request()does one or twostatcalls under the file lock.Limits
Tests
test/e2e/cache_files_changed_test.dart(replacespartial_deleted_test.dart):test/e2e/reset_cache_test.dart:resetCache()while the download waits for the origin.test/io/buffered_io_sink_test.dart: resuming a deleted, truncated or written-to file fails without writing to it.test/unit/cache_state_test.dart:filePositiondefaults and equality.flutter test: 156 pass, in 3 runs.dart analyze lib test: no issues.flutter analyze: only the existing issues inbenchmarker/.trail checkflags three lines (cache_downloader.dart:168,http_cache_stream.dart:380,cache_files_changed_test.dart:57) as bringing back what the learning saved for this work ruled out. They are the fixes that learning records (the block-bodied flush error handler, the cancel under the lock), not the versions it ruled out.